-
Notifications
You must be signed in to change notification settings - Fork 0
KFuzzTest PR v2 #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
KFuzzTest PR v2 #15
Conversation
0d459e6 to
cf958c0
Compare
lib/kfuzztest/main.c
Outdated
| * | ||
| * @kfuzztest_dir: The root debugfs directory, /sys/kernel/debug/kfuzztest/. | ||
| * @num_invocations: total number of target invocations. | ||
| * @num_targets: number registered targets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"number of register targets"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the number of targets that have been registered with KFuzzTest - I.e. that have their debugfs entries created. I could maybe change the comment here, in favor of something like "number of created targets"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is that the comment is missing a preposition :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhhh gotcha. Whoops yeah it totally is
f4530fc to
a227cde
Compare
| } | ||
|
|
||
| if (token_count == token_arr_size) { | ||
| token_arr_size *= 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using smarter realloc (by doubling the size every time we need to grow) but I'm doing it "stupider" in other places by just incrementing by one.
It's clear that this is more efficient, and it might be worth doing this in other locations, but it isn't really important the performance overhead will be basically nothing seeing as the function invokes a system call anyways.
For consistency though, might be worth doing this similarly everywhere.
0251749 to
be47f56
Compare
3aa4235 to
3f1ac1c
Compare
| err = parse(tokens, num_tokens, &ast_prog); | ||
| if (err) { | ||
| fprintf(stderr, "parsing failed: %s\n", strerror(-err)); | ||
| goto cleanup_tokens; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: if your cleanup functions handle the NULL pointers correctly, you can probably use a single cleanup label for every goto statement.
This is purely optional.
lib/kfuzztest/main.c
Outdated
| * | ||
| * @kfuzztest_dir: The root debugfs directory, /sys/kernel/debug/kfuzztest/. | ||
| * @num_invocations: total number of target invocations. | ||
| * @num_targets: number registered targets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is that the comment is missing a preposition :)
c1648c2 to
e50764e
Compare
Introduce a new helper function, kasan_poison_range(), to encapsulate the logic for poisoning an arbitrary memory range of a given size, and expose it publically in <include/linux/kasan.h>. This is a preparatory change for the upcoming KFuzzTest patches, which requires the ability to poison the inter-region padding in its input buffers. No functional change to any other subsystem is intended by this commit. Signed-off-by: Ethan Graham <ethangraham@google.com> Reviewed-by: Alexander Potapenko <glider@google.com> --- PR v1: - Enforce KASAN_GRANULE_SIZE alignment for the end of the range in kasan_poison_range(), and return -EINVAL when this isn't respected. ---
Add the foundational user-facing components for the KFuzzTest framework. This includes the main API header <linux/kfuzztest.h>, the Kconfig option to enable the feature, and the required linker script changes which introduce three new ELF sections in vmlinux. Note that KFuzzTest is intended strictly for debug builds only, and should never be enabled in a production build. The fact that it exposes internal kernel functions and state directly to userspace may constitute a serious security vulnerability if used for any reason other than testing. The header defines: - The FUZZ_TEST() macro for creating test targets. - The data structures required for the binary serialization format, which allows passing complex inputs from userspace. - The metadata structures for test targets, constraints and annotations, which are placed in dedicated ELF sections (.kfuzztest_*) for discovery. This patch only adds the public interface and build integration; no runtime logic is included. Signed-off-by: Ethan Graham <ethangraham@google.com> --- PR v1: - Move KFuzzTest metadata definitions to generic vmlinux linkage so that the framework isn't bound to x86_64. - Return -EFAULT when simple_write_to_buffer returns a value not equal to the input length in the main FUZZ_TEST macro. - Enforce a maximum input size of 64KiB in the main FUZZ_TEST macro, returning -EINVAL when it isn't respected. - Refactor KFUZZTEST_ANNOTATION_* macros. - Taint the kernel with TAINT_TEST inside the FUZZ_TEST macro when a fuzz target is invoked for the first time. ---
Add the core runtime implementation for KFuzzTest. This includes the module initialization, and the logic for receiving and processing user-provided inputs through debugfs. On module load, the framework discovers all test targets by iterating over the .kfuzztest_target section, creating a corresponding debugfs directory with a write-only 'input' file for each of them. Writing to an 'input' file triggers the main fuzzing sequence: 1. The serialized input is copied from userspace into a kernel buffer. 2. The buffer is parsed to validate the region array and relocation table. 3. Pointers are patched based on the relocation entries, and in KASAN builds the inter-region padding is poisoned. 4. The resulting struct is passed to the user-defined test logic. Signed-off-by: Ethan Graham <ethangraham@google.com> --- PR v2: - Fix build issues identified by the kernel test robot <lkp@intel.com>. - Address some nits pointed out by Alexander Potapenko. PR v1: - Update kfuzztest/parse.c interfaces to take `unsigned char *` instead of `void *`, reducing the number of pointer casts. - Expose minimum region alignment via a new debugfs file. - Expose number of successful invocations via a new debugfs file. - Refactor module init function, add _config directory with entries containing KFuzzTest state information. - Account for kasan_poison_range() return value in input parsing logic. - Validate alignment of payload end. - Move static sizeof assertions into /lib/kfuzztest/main.c. - Remove the taint in kfuzztest/main.c. We instead taint the kernel as soon as a fuzz test is invoked for the first time, which is done in the primary FUZZ_TEST macro. RFC v2: - The module's init function now taints the kernel with TAINT_TEST. ---
Introduce the kfuzztest-bridge tool, a userspace utility for sending structured inputs to KFuzzTest harnesses via debugfs. The bridge takes a textual description of the expected input format, a file containing random bytes, and the name of the target fuzz test. It parses the description, encodes the random data into the binary format expected by the kernel, and writes the result to the corresponding debugfs entry. This allows for both simple manual testing and integration with userspace fuzzing engines. For example, it can be used for smoke testing by providing data from /dev/urandom, or act as a bridge for blob-based fuzzers (e.g., AFL) to target KFuzzTest harnesses. Signed-off-by: Ethan Graham <ethangraham@google.com> --- PR v2: - Move kfuzztest-bridge tool under tools/testing, as suggested by SeongJae Park. - Cleanup several resource leaks that were pointed out by Alexander Potapenko. PR v1: - Add additional context in header comment of kfuzztest-bridge/parser.c. - Add some missing NULL checks. - Refactor skip_whitespace() function in input_lexer.c. - Use ctx->minalign to compute correct region alignment, which is read from /sys/kernel/debug/kfuzztest/_config/minalign. ---
Add Documentation/dev-tools/kfuzztest.rst and reference it in the dev-tools index. Signed-off-by: Ethan Graham <ethangraham@google.com> Acked-by: Alexander Potapenko <glider@google.com> --- PR v2: - Update documentation to reflect new location of kfuzztest-bridge, under tools/testing. PR v1: - Fix some typos and reword some sections. - Correct kfuzztest-bridge grammar description. - Reference documentation in kfuzztest-bridge/input_parser.c header comment. RFC v2: - Add documentation for kfuzztest-bridge tool introduced in patch 4. ---
Add two simple fuzz target samples to demonstrate the KFuzzTest API and provide basic self-tests for the framework. These examples showcase how a developer can define a fuzz target using the FUZZ_TEST(), constraint, and annotation macros, and serve as runtime sanity checks for the core logic. For example, they test that out-of-bounds memory accesses into poisoned padding regions are correctly detected in a KASAN build. These have been tested by writing syzkaller-generated inputs into their debugfs 'input' files and verifying that the correct KASAN reports were triggered. Signed-off-by: Ethan Graham <ethangraham@google.com> Acked-by: Alexander Potapenko <glider@google.com> --- PR v2: - Fix build issues pointed out by the kernel test robot <lkp@intel.com>. ---
Add KFuzzTest targets for pkcs7_parse_message, rsa_parse_pub_key, and rsa_parse_priv_key to serve as real-world examples of how the framework is used. These functions are ideal candidates for KFuzzTest as they perform complex parsing of user-controlled data but are not directly exposed at the syscall boundary. This makes them difficult to exercise with traditional fuzzing tools and showcases the primary strength of the KFuzzTest framework: providing an interface to fuzz internal functions. To validate the effectiveness of the framework on these new targets, we injected two artificial bugs and let syzkaller fuzz the targets in an attempt to catch them. The first of these was calling the asn1 decoder with an incorrect input from pkcs7_parse_message, like so: - ret = asn1_ber_decoder(&pkcs7_decoder, ctx, data, datalen); + ret = asn1_ber_decoder(&pkcs7_decoder, ctx, data, datalen + 1); The second was bug deeper inside of asn1_ber_decoder itself, like so: - for (len = 0; n > 0; n--) + for (len = 0; n >= 0; n--) syzkaller was able to trigger these bugs, and the associated KASAN slab-out-of-bounds reports, within seconds. The targets are defined within crypto/asymmetric-keys/tests. Signed-off-by: Ethan Graham <ethangraham@google.com> Reviewed-by: Ignat Korchagin <ignat@cloudflare.com> --- PR v2: - Make fuzz targets also depend on the KConfig options needed for the functions they are fuzzing, CONFIG_PKCS7_MESSAGE_PARSER and CONFIG_CRYPTO_RSA respectively. - Fix build issues pointed out by the kernel test robot <lkp@intel.com>. - Account for return value of pkcs7_parse_message, and free resources if the function call succeeds. PR v1: - Change the fuzz target build to depend on CONFIG_KFUZZTEST=y, eliminating the need for a separate config option for each individual file as suggested by Ignat Korchagin. - Remove KFUZZTEST_EXPECT_LE on the length of the `key` field inside of the fuzz targets. A maximum length is now set inside of the core input parsing logic. RFC v2: - Move KFuzzTest targets outside of the source files into dedicated _kfuzz.c files under /crypto/asymmetric_keys/tests/ as suggested by Ignat Korchagin and Eric Biggers. ---
Add a KFuzzTest fuzzer for the parse_xy() function, located in a new file under /drivers/auxdisplay/tests. To validate the correctness and effectiveness of this KFuzzTest target, a bug was injected into parse_xy() like so: drivers/auxdisplay/charlcd.c:179 - s = p; + s = p + 1; Although a simple off-by-one bug, it requires a specific input sequence in order to trigger it, thus demonstrating the power of pairing KFuzzTest with a coverage-guided fuzzer like syzkaller. Signed-off-by: Ethan Graham <ethangraham@google.com>
Add a KFuzzTest target for the load_script function to serve as a real-world example of the framework's usage. The load_script function is responsible for parsing the shebang line (`#!`) of script files. This makes it an excellent candidate for KFuzzTest, as it involves parsing user-controlled data within the binary loading path, which is not directly exposed as a system call. The provided fuzz target in fs/tests/binfmt_script_kfuzz.c illustrates how to fuzz a function that requires more involved setup - here, we only let the fuzzer generate input for the `buf` field of struct linux_bprm, and manually set the other fields with sensible values inside of the FUZZ_TEST body. To demonstrate the effectiveness of the fuzz target, a buffer overflow bug was injected in the load_script function like so: - buf_end = bprm->buf + sizeof(bprm->buf) - 1; + buf_end = bprm->buf + sizeof(bprm->buf) + 1; Which was caught in around 40 seconds by syzkaller simultaneously fuzzing four other targets, a realistic use case where targets are continuously fuzzed. It also requires that the fuzzer be smart enough to generate an input starting with `#!`. While this bug is shallow, the fact that the bug is caught quickly and with minimal additional code can potentially be a source of confidence when modifying existing implementations or writing new functions. Signed-off-by: Ethan Graham <ethangraham@google.com> --- PR v2: - Introduce cleanup logic in the load_script fuzz target. ---
Add myself as maintainer and Alexander Potapenko as reviewer for KFuzzTest. Signed-off-by: Ethan Graham <ethangraham@google.com> Acked-by: Alexander Potapenko <glider@google.com>
e50764e to
89394c2
Compare
No description provided.